Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add analysis metrics menu to chart #860

Merged
merged 2 commits into from
Feb 29, 2024

Conversation

sandrahoang686
Copy link
Collaborator

@sandrahoang686 sandrahoang686 commented Feb 27, 2024

This PR adds the analysis metrics menu back to each layer list item in the explorations page.

Ticket here
Design here

Question for @faustoperez related to icon button location. I moved the button closer to the right-aligned because of the overlapping arrows. Please let me know what you think. I discuss this more in the demo video if you can take a look. Thanks!

Demo vid:
https://www.loom.com/share/56d013aade7945fdb40bff8cd2f3dc56

Copy link

netlify bot commented Feb 27, 2024

Deploy Preview for veda-ui ready!

Name Link
🔨 Latest commit 9c3eefd
🔍 Latest deploy log https://app.netlify.com/sites/veda-ui/deploys/65df6c2b0949b80008ac1298
😎 Deploy Preview https://deploy-preview-860--veda-ui.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

@faustoperez faustoperez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Sandra, I love the videos! Good call moving the icon to the right so it doesn't overlap the chart.

Could you please move it together with the units a little bit to the left (say 4-8px) so it has a bit of space? It's a little bit too tight now. Thanks!

image

@faustoperez
Copy link

As a side note related to this PR @j08lue @sandrahoang686 we need to review the behavior of the interface after triggering an analysis. I suggest:

  • Fly and zoom the map to AOI
  • Zoom the timeline to focus on the selected analysis period

I suggest we tackle that on #857

@sandrahoang686 sandrahoang686 changed the title [#846] Add analysis metrics menu to chart Add analysis metrics menu to chart Feb 28, 2024
Copy link
Contributor

@j08lue j08lue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Metrics selection works well with the new menu. I am not sure how well it can be found or we should use an icon that hints more at that these are chart-related options, now that the icon is not floating above the lines it is related to, but we can see in user testing, whether that is an issue and address it together with other planned design changes, if need be.

Copy link

@faustoperez faustoperez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's ok for now, might review the design at a later stage 👍 Thanks Sandra!

@sandrahoang686 sandrahoang686 merged commit e6e3cf2 into main Feb 29, 2024
8 checks passed
@sandrahoang686 sandrahoang686 deleted the update/ae-add-stats-menu-to-chart branch February 29, 2024 14:32
hanbyul-here added a commit that referenced this pull request Mar 25, 2024
## 🎉 Features
- Optional media attributes for layers:
#843
- Add custom javascript injection
#846
- ADR for V2 Refactor: #875

## 🚀 Improvements
- E&A imporvement. Related tickets
  - Layer select modal: #845
- Connect dataset information on layer:
#821
  - Layer info modal: #849
- Update data layer card:
#851
  - Hidden layers: #867
- Fast follow-ups: #851 ,
#862,
#863,
#860
  - PR template: #880


## 🐛 Fixes
- Return datasets even when there is a dataset without summaries:
#786
- Show all the datasets on Data Catalog page:
#837
- Block Map user defined position fix:
#784
- Geocoder centering on various projecctions:
#826
- Wording, typo: #869
#854,
#874,
- Fix yaxis labeling: #883
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants